Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

kubeadm/cilium: bump tested versions #291

Merged
merged 4 commits into from
Feb 15, 2022
Merged

Conversation

tormath1
Copy link
Contributor

@tormath1 tormath1 commented Feb 14, 2022

In this PR, we bump the cilium version to latest v1.11.0 and we give the ability to set the version from the cilium-cli directly, since it has been introduced in the CLI.

This PR aims to have a more recent version of the tested cilium in order to prepare the introduction of a new test to validate the behavior of IPSec + cilium.

Testing done

  • tests on EM with kubeadm.v1.23.0.cilium.base
  • Changelog entries added in the respective changelog/ directory (user-facing change, bug fix, security fix, update)

Signed-off-by: Mathieu Tortuyaux <[email protected]>
in previous cilium-cli version, it was not possible to set the Cilium
version from the CLI, we were relying on the compiled default version.

it's now easier to update cilium itself and to know which version is
actually being tested.

Signed-off-by: Mathieu Tortuyaux <[email protected]>
we assert that the cilium install script is correctly loaded with the
cilium version.

Signed-off-by: Mathieu Tortuyaux <[email protected]>
@tormath1 tormath1 self-assigned this Feb 14, 2022
@tormath1 tormath1 marked this pull request as ready for review February 14, 2022 14:49
@tormath1 tormath1 requested a review from a team February 14, 2022 14:49
Copy link
Member

@krnowak krnowak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I grepped for cilium and found kola/tests/kubeadm/testdata/master-cilium-amd64-config.yml:102: url: https://github.com/cilium/cilium-cli/releases/download/v0.9.0/cilium-linux-amd64.tar.gz. Should version be also updated there?

CHANGELOG.md Outdated
@@ -19,6 +19,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
- removed `packet` occurrences in favor of `equinixmetal` ([#277](https://github.com/flatcar-linux/mantle/pull/277))
- kola: fixed cl.filesystem test for systemd 250 and newer ([#280](https://github.com/flatcar-linux/mantle/pull/280))
- PXE boots now over HTTPS on Equinix Metal ([#288](https://github.com/flatcar-linux/mantle/pull/288))
- cilium tested version to 1.11.1 ([291](https://github.com/flatcar-linux/mantle/pull/291))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing a verb, like "bump"?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah that's a good point actually.

I personally have the habit to read section from title to the bullet point. So

## Changed

- cilium tested version to 1.11.1

can be read as "changed cilium tested version to 1.11.1". It avoids redundancy like:

## Changed

- Changed cilium tested version to 1.11.1

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah that's a good point actually.

I personally have the habit to read section from title to the bullet point. So

## Changed

- cilium tested version to 1.11.1

can be read as "changed cilium tested version to 1.11.1".

Clearly the other entries are not following your habit. :) Not saying that your habit is bad or wrong, but the entry looks odd without a verb.

It avoids redundancy like:

## Changed

- Changed cilium tested version to 1.11.1

Yeah, I'd use some more specific verb than "changed", like "bumped" or "updated".

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, let's do that then !

@tormath1
Copy link
Contributor Author

I grepped for cilium and found kola/tests/kubeadm/testdata/master-cilium-amd64-config.yml:102: url: https://github.com/cilium/cilium-cli/releases/download/v0.9.0/cilium-linux-amd64.tar.gz. Should version be also updated there?

That's not an issue, it's a testdata file used to unittest the template generation:
https://github.com/flatcar-linux/mantle/blob/2f39bbdd42f8bed7e3e729955a1dfc43aaf1c519/kola/tests/kubeadm/kubeadm_test.go#L89

We only check that uploaded templates on worker/controller are correctly rendered.

@krnowak
Copy link
Member

krnowak commented Feb 14, 2022

I grepped for cilium and found kola/tests/kubeadm/testdata/master-cilium-amd64-config.yml:102: url: https://github.com/cilium/cilium-cli/releases/download/v0.9.0/cilium-linux-amd64.tar.gz. Should version be also updated there?

That's not an issue, it's a testdata file used to unittest the template generation:

https://github.com/flatcar-linux/mantle/blob/2f39bbdd42f8bed7e3e729955a1dfc43aaf1c519/kola/tests/kubeadm/kubeadm_test.go#L89

We only check that uploaded templates on worker/controller are correctly rendered.

Ok, thanks for checking!

Signed-off-by: Mathieu Tortuyaux <[email protected]>
@tormath1 tormath1 merged commit 697b99a into flatcar-master Feb 15, 2022
@tormath1 tormath1 deleted the tormath1/cilium branch February 15, 2022 13:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants